Skip to content

[#739] Fix plot_count +1 bug: reconcile after genesis plot#741

Merged
realproject7 merged 3 commits intomainfrom
task/739-plot-count-bug
Apr 2, 2026
Merged

[#739] Fix plot_count +1 bug: reconcile after genesis plot#741
realproject7 merged 3 commits intomainfrom
task/739-plot-count-bug

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Root cause: StorylineCreated handler hardcodes plot_count: 1 but never calls reconcileStorylinePlotCount(), unlike the PlotChained handler which does. This causes the cached plot_count to drift by +1.
  • Fix: Add reconcileStorylinePlotCount() after genesis plot upsert in both:
    • src/app/api/index/storyline/route.ts (live indexer)
    • src/app/api/cron/backfill/route.ts (backfill cron)
  • Why Reading Mode was correct: It counts actual chapter objects, not the cached plot_count field.
  • All display surfaces (profile, story page, cards, OG images) use storyline.plot_count from DB — no UI changes needed.

Fixes #739

Test plan

  • Verify existing storylines show correct plot count after a backfill run
  • Create a new storyline — plot_count should be 1 (not 2)
  • Chain a second plot — plot_count should be 2
  • Reading Mode count matches profile/story page count
  • npm run build passes

🤖 Generated with Claude Code

Root cause: StorylineCreated handler (indexer + backfill) hardcodes
plot_count: 1 but never calls reconcileStorylinePlotCount(), unlike
the PlotChained handler which does. This causes drift when genesis
plots are processed, leading to plot_count being off by +1.

Fix: Add reconcileStorylinePlotCount() call after genesis plot upsert
in both the indexer (index/storyline/route.ts) and the backfill cron
(cron/backfill/route.ts), matching the existing pattern used by the
PlotChained handler.

Reading Mode was unaffected because it counts actual chapter objects
rather than relying on the cached plot_count field.

Fixes #739

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
plotlink Ignored Ignored Apr 2, 2026 7:48am

Request Review

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T2b APPROVE — root cause and fix verified:

Root cause: StorylineCreated hardcodes plot_count:1 then inserts genesis plot, but never reconciles — unlike PlotChained which calls reconcileStorylinePlotCount(). Result: cached plot_count drifts +1.

Fix: Add reconcileStorylinePlotCount() after genesis plot upsert in both:

  • src/app/api/index/storyline/route.ts (live indexer) ✓
  • src/app/api/cron/backfill/route.ts (backfill cron) ✓

Import added only where missing (storyline route); backfill already had it. Reuses existing reconcile function — no new abstractions. Reading Mode unaffected (counts chapters directly). +7 lines, 0 deletions. Minimal and correct.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The root-cause analysis looks right, but this patch only fixes future StorylineCreated ingests. Existing storylines with inflated plot_count values will stay wrong after merge.

Findings

  • [high] The backfill cron still resumes from backfill_cursor.last_block, so the new reconcileStorylinePlotCount() only runs for brand-new or explicitly replayed StorylineCreated events. Production storylines that were already indexed before this patch are never revisited, which means the globally wrong storyline.plot_count values on profile/story/card surfaces remain wrong even after deployment.
    • File: src/app/api/cron/backfill/route.ts:110
    • Suggestion: add an explicit one-off reconciliation for existing storylines, or intentionally replay the affected historical range as part of the fix so current rows are corrected, not just future inserts.

Decision

Requesting changes because issue #739 is a global existing-data bug, and this PR currently prevents new drift without repairing the rows that are already wrong.

Adds POST /api/cron/reconcile-plots that iterates all storylines and
reconciles plot_count from the actual plots table. Idempotent and safe
to run multiple times. Fixes inflated plot_count on already-indexed
storylines that were affected before the genesis reconciliation fix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The new reconcile endpoint repairs existing bad rows, but it is exposed without the cron authorization guard used by the other maintenance routes.

Findings

  • [high] POST /api/cron/reconcile-plots is unauthenticated. Unlike the existing cron/backfill route, this new endpoint does not check CRON_SECRET or any auth header before iterating every storyline and issuing server-side updates. That means any external caller can trigger a full-database reconciliation job on demand.
    • File: src/app/api/cron/reconcile-plots/route.ts:12
    • Suggestion: apply the same cron authorization pattern used in src/app/api/cron/backfill/route.ts before running the reconciliation loop.

Decision

Requesting changes because the data-fix direction is right, but the new maintenance endpoint needs the existing cron auth guard before it is safe to merge.

Matches the same auth pattern used by the backfill cron route.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The code-level blockers are resolved, but issue #739 still has no ticket comment documenting the root cause.

Findings

  • [medium] Issue #739 explicitly requires the root cause to be documented in the ticket comments, but the issue currently has no comments. The PR summary explains it, but the acceptance criterion is still unmet.
    • File: issue #739 comments
    • Suggestion: post the root-cause summary to issue #739 before merge so the fix is documented where the ticket asked for it.

Decision

Keeping this as request changes because the implementation looks correct now, but the ticket’s documentation requirement is still incomplete.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The remaining documentation blocker is resolved. Issue #739 now has the required root-cause comment, and the code changes cover both future ingests and existing storyline reconciliation.

Findings

  • None.

Decision

Approving because the review blockers are addressed and the ticket requirements are now complete.

@realproject7 realproject7 merged commit 54fa052 into main Apr 2, 2026
5 checks passed
@realproject7 realproject7 deleted the task/739-plot-count-bug branch April 2, 2026 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix plot_count +1 bug — genesis plot counted twice globally

2 participants